Repository navigation
feat: report host OS context under Wine, Proton and Linux containers - #2241
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v1 #2241 +/- ##
==========================================
- Coverage 76.28% 76.22% -0.06%
==========================================
Files 104 104
Lines 29717 29813 +96
Branches 5504 5530 +26
==========================================
+ Hits 22670 22726 +56
- Misses 5671 5701 +30
- Partials 1376 1386 +10 🚀 New features to boost your workflow:
|
|
What do you think about targeting |
Under Wine, build the OS context from the host via wine_get_host_version and the host os-release read through the Z: drive, in the same shape as the native Linux OS context. Prefer /run/host to report the host instead of the Steam Runtime or Flatpak container.
31d10d1 to
756c4ee
Compare
Good call, re-targeted it to v1. |
|
I'm happy with if both Unreal and Godot are happy with this 😏 |
Removed the 'Features' section from the CHANGELOG.
|
|
||
| build[0] = '\0'; | ||
|
|
||
| sentry_value_set_by_key(os, "name", sentry_value_new_string(sysname)); |
There was a problem hiding this comment.
Both Godot and Unreal currently override OS name for SteamOS and Bazzite, with the distro release in os.version. This implementation would report Linux and the kernel version instead.
Since the PR is intended to replace those overrides, should we bring on that specific naming promotion to native?
cc @mujacica
There was a problem hiding this comment.
Makes sense, otherwise we’d still need to do the parsing in both SDKs.
There was a problem hiding this comment.
Alright, added special handling for SteamOS and Bazzite similar to what Unreal/Godot already do. Not sure if other distros need the same treatment but the list is easy to extend if that ever comes up.
tustanivsky
changed the title
There was a problem hiding this comment.
Should we adapt these assertions for named OS distributions? In case someone runs these tests on such OS.
There was a problem hiding this comment.
Added special handling for named distros.
Under Wine and Proton, the OS context currently reports the Windows version that Wine emulates (e.g.
Windows 10.0.19045). This reports the host OS instead, in the same shape as the native Linux OS context, next to thewinecontext added in #1995.wine_get_host_versionexport, and split them intoname,versionandbuildlike on Linuxos-releasefor thedistribution_*fields (through theZ:drive under Wine), preferring/run/hostso the host is reported instead of the Steam Runtime or Flatpak container. This applies to native Linux toonamefromNAME,versionfromVERSION_ID,buildfromBUILD_ID, with the kernel release inkernel_versionos-releaseparser between the Linux and Windows codeBehavior change
os.namechanges fromWindowsto the host OS (Linux,SteamOS,Bazzite, orDarwinon macOS hosts), and theos.name/os.versionattributes of logs and metrics change with it.distribution_*now reports the host distro instead of the container.SteamOS 3.7instead ofLinux 6.xwithdistribution_name: steamos.This matches what sentry-godot and sentry-unreal already report via their own OS context overrides, which this PR is meant to replace.
The emulated Windows version is no longer reported. If it is useful, it could be added to the
winecontext (e.g.windows_version).Related